Fix "save as" operation for the data folder of a sketch #6041
Conversation
|
|
||
| // Copy the data folder (this may take a while.. add progress bar?) | ||
| if (getDataFolder().exists()) { | ||
| File newDataFolder = new File(newFolder, "data"); | ||
| // Create the data folder | ||
| if (!newDataFolder.mkdirs()) { |
There was a problem hiding this comment.
Should we also check for newDataFolder.exists() before bailing out? The check could be probably turned into if(!(newDataFolder.exists() || newDataFolder.mkdirs())) . Do you see any drawback in this approach?
There was a problem hiding this comment.
We can check if the folder exists, but since we just created the parent folder at line 341, it is very unlikely the folder already exits.
Never the less it won't harm if we implement this extra check. So I will add the check and test if it still works.
|
Changes look good to me. The first and last commit should probably be dropped, no point in polluting the history with them. As for the second commit (moving the |
| // Create the data folder | ||
| if (!newDataFolder.mkdirs()) { | ||
| // Check if data folder exits, if not try to create the data folder | ||
| if (!(newDataFolder.exists() || newDataFolder.mkdirs())) { |
There was a problem hiding this comment.
I would think that if (!newDataFolder.exists() && !newDataFolder.mkdirs()) is a bit easier to read, would you agree?
There was a problem hiding this comment.
For readability you might even want to separate the condition into two if statements. (In case you don't know how the lazy and/or operator works).
But I think it is better to keep them into one if statement. I will change it to the and operator for better readability.
Changed the location where the variable `folder` gets updated. The function `getDataFolder()` uses this variable to return the data folder. It was looking for the data folder of the original sketch in the folder of the new created sketch. Furthermore the data folder will now be created if it does not exist yet in the new sketch before copying the files of the original sketch.
jeroenoverman
force-pushed
the
bug/data-folder
branch
from
cc99a67 to
7714a41
Compare
|
I have fixed the comments you gave me. |
|
@facchinm Is this Pull Request ready to be merged or should something still be changed or more tested? |
|
Looks fine, thanks for the ping 😉 |
This pull fixes #5998.
When performing a save as operation the data folder of the sketch was not copied to the new location.
What has changed:
I have tested this on my Windows machine and it solves the issue.